Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix/smaller improvements #1823

Merged
merged 8 commits into from
Nov 13, 2024
Merged

Fix/smaller improvements #1823

merged 8 commits into from
Nov 13, 2024

Conversation

noahonyejese
Copy link
Contributor

@noahonyejese noahonyejese commented Nov 8, 2024

This PR resolves #1818 , resolves #1800

  • This PR adds sharing and embedding capabilities directly into the users profile
  • This PR improves the visibiliy current dataset used in during Visualization Creation
  • This PR resolves the overlapping bug when using interactive Filters

Next Steps

  1. @KerstinFaye please provide a design for ✓ Improve visibility of selected data set #1800 and ✓Allow User to Copy Share Link from User Profile #1758 like this I can proceed. More details about ideas from the Team you can find under the Issue's themselves ✅

How to Test (1800)

  1. Go to this Link
  2. Create a Visualization (f.e Einmalvergütung für Photovoltaikanlagen)
  3. See how the Data Sources Section is Called Datasets ✅
  4. See how the Datasets Section is all the way on the top ✅
  5. See how button styling within the Datasets Section changed from outlined to text ✅

How to Test (1818)

  1. Go to this Link
  2. Create a new Visualization
  3. Select "Columns" or "Areas" as the chart type.
  4. Add a segmentation column in the Chart options.
  5. Change the Chart mode to interactive
  6. Observe the overlapping elements.

Test how it was before
old Issue Link


How to Test (1758)

Copy link

vercel bot commented Nov 8, 2024

The latest updates on your projects. Learn more about Vercel for Git ↗︎

Name Status Preview Comments Updated (UTC)
visualization-tool ❌ Failed (Inspect) Nov 13, 2024 5:10am

@sosiology
Copy link
Contributor

@noahonyejese thanks bug with the interactive filters LGTM! 👏

Will test:

  • sharing and embedding capabilities directly into the users profile once on TEST as i cannot login to my profile on a PR
  • visibiliy current dataset used in during Visualization Creation once it is implemented as designed.

@noahonyejese
Copy link
Contributor Author

@noahonyejese thanks bug with the interactive filters LGTM! 👏

Will test:

  • sharing and embedding capabilities directly into the users profile once on TEST as i cannot login to my profile on a PR
  • visibiliy current dataset used in during Visualization Creation once it is implemented as designed.

Hi @sosiology the #1800 is implemented based on @KerstinFaye design Please feel free to compare.

@noahonyejese
Copy link
Contributor Author

Proposals: @KerstinFaye should we remove the Beta flag or the notification (1) from the section I think especially the (1) is drawing enormous attention towards it causing it to be opened even tho this may not be something the user will want to use straight away?

Copy link
Collaborator

@bprusinowski bprusinowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks 💯

app/charts/shared/containers.tsx Outdated Show resolved Hide resolved
app/components/publish-actions.tsx Outdated Show resolved Hide resolved
app/components/publish-actions.tsx Show resolved Hide resolved
app/components/publish-actions.tsx Outdated Show resolved Hide resolved
app/locales/de/messages.po Show resolved Hide resolved
app/locales/en/messages.po Show resolved Hide resolved
app/configurator/components/dataset-control-section.tsx Outdated Show resolved Hide resolved
@sosiology
Copy link
Contributor

@noahonyejese for #1800 i noticed that the data sources are now shown at the top but the accordion is closed per default - so it still requires user interaction (opening the accordion) before the data set selected is visible, can we per default show the accordion open?

@noahonyejese noahonyejese merged commit e5170fa into main Nov 13, 2024
3 of 5 checks passed
@noahonyejese noahonyejese deleted the fix/smaller-improvements branch November 13, 2024 05:06
@noahonyejese
Copy link
Contributor Author

@bprusinowski I will be wrapping this up a new PR

@KerstinFaye
Copy link

Proposals: @KerstinFaye should we remove the Beta flag or the notification (1) from the section I think especially the (1) is drawing enormous attention towards it causing it to be opened even tho this may not be something the user will want to use straight away?

@noahonyejese thanks Noah, yes. I think we could remove it now. @bprusinowski, @sosiology any opinion on this?

@bprusinowski
Copy link
Collaborator

bprusinowski commented Nov 15, 2024

@KerstinFaye I would wait until we finalize merging of cubes improvements. As we realistically won't have a new PROD release before they are completed, should be fine to wait a bit?

We still need to wrap up #1849 and the grouped filters, drop-down options and table charts, right?

Or we could keep it in other places (modal to add a dataset), just remove the label from Datasets section 🤔

@sosiology
Copy link
Contributor

Agree to keep the beta label until we have completed the improvements. Also, we are still waiting on feedback about the proposed placement at the top of the sidebar.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
4 participants